fix(codex): preserve wrapped sessions and recover state#2160
Conversation
PR governanceThis PR follows the template and is marked ready for human review. |
JerrettDavis
left a comment
There was a problem hiding this comment.
Reviewed current head e6dc3af. This is a large Codex behavior change, but the durable-home launch path preserves provider identity through process-local config overrides, and the recovery flow now has transactional backups plus explicit SQLite handle closure before rollback restore. I fixed the Windows rollback/test portability issues and verified locally: uvx ruff@0.15.17 check/format for the Codex wrap/recovery files, and uv run --extra dev pytest tests/test_cli/test_wrap_codex.py tests/test_cli/test_recover_codex.py -q (107 passed, 1 skipped). CI is rerunning on this head.
JerrettDavis
left a comment
There was a problem hiding this comment.
Validated current head 16bce6e after merging latest headroomlabs/main and resolving the uv.lock Pillow floor conflict in favor of the upstream security floor.
Checks run:
uv lock --checkgit diff --checkuvx ruff@0.15.17 check headroom/cli/recover.py headroom/cli/wrap.py headroom/providers/codex/recovery.py tests/test_cli/test_recover_codex.py tests/test_cli/test_wrap_codex.pyuvx ruff@0.15.17 format --check headroom/cli/recover.py headroom/cli/wrap.py headroom/providers/codex/recovery.py tests/test_cli/test_recover_codex.py tests/test_cli/test_wrap_codex.pyuv run --extra dev pytest tests/test_cli/test_wrap_codex.py tests/test_cli/test_recover_codex.py -q(107 passed, 1 skipped)
JerrettDavis
left a comment
There was a problem hiding this comment.
Approved current head 59f4162 after adding the Windows-safe SQLite rollback handle cleanup.
Validated locally:
uv run --extra dev python -m pytest tests/test_cli/test_recover_codex.py::test_recovery_rolls_back_when_source_sqlite_breaks_foreign_keys -quv run --extra dev python -m pytest tests/test_cli/test_recover_codex.py tests/test_cli/test_wrap_codex.py -quvx ruff@0.15.17 check headroom/cli/recover.py headroom/cli/wrap.py headroom/providers/codex/recovery.py tests/test_cli/test_recover_codex.py tests/test_cli/test_wrap_codex.pyuvx ruff@0.15.17 format --check headroom/cli/recover.py headroom/cli/wrap.py headroom/providers/codex/recovery.py tests/test_cli/test_recover_codex.py tests/test_cli/test_wrap_codex.pyuvx mypy@1.20.2 headroom/providers/codex/recovery.py headroom/cli/recover.py --ignore-missing-importsgit diff --check
95a8c11 to
d1e000e
Compare
40103ce to
247239b
Compare
JerrettDavis
left a comment
There was a problem hiding this comment.
Reviewed the new provider-normalization commit 26540c2. The rollout/session_meta rewrite is scoped to legacy localhost headroom routing, preserves remote providers named headroom, and keeps newer target rollouts intact while repairing the stale provider marker.
Validated locally on the PR head:
uv run --extra dev python -m pytest tests/test_cli/test_recover_codex.py::test_recovery_restores_legacy_headroom_threads_to_active_provider tests/test_cli/test_recover_codex.py::test_recovery_repairs_legacy_provider_after_a_previous_broken_recovery tests/test_cli/test_recover_codex.py::test_recovery_preserves_nonlocal_provider_named_headroom -quvx ruff@0.15.17 check headroom/providers/codex/recovery.py tests/test_cli/test_recover_codex.py
The visible red checks are still the shared dependency audit / release-smoke baseline being addressed separately by #2190.
26540c2 to
3c9b8ee
Compare
f166f51 to
2d89ece
Compare
|
@JerrettDavis, @chopratejas @DevanshiVyas I would like you to do a careful review on this PR, it modifies the codex sessions database in order to restore the dangling headroom wrapped sessions. |
JerrettDavis
left a comment
There was a problem hiding this comment.
Re-reviewed the latest head (2d89ecec) after the post-approval recovery hardening commits.
I focused on the risky parts: durable CODEX_HOME launch behavior, process-local provider routing for OpenAI and custom providers, recovery rollback, SQLite schema / migration / FK validation, target symlink traversal, runtime socket/FIFO skipping, legacy localhost headroom provider normalization, and preservation of user-defined remote providers named headroom.
The implementation is still conservative where it needs to be: it pins the source before merging, backs up the target, refuses overlapping homes, rejects writes through target symlinks, validates SQLite shape before merging rows, and keeps the new launch routing out of persistent Codex provider config.
Verified locally on Windows/Python 3.13:
uv run pytest tests/test_cli/test_wrap_codex.py tests/test_cli/test_recover_codex.py -q
121 passed, 1 skipped
uv run ruff check headroom/cli/recover.py headroom/providers/codex/recovery.py tests/test_cli/test_recover_codex.py tests/test_cli/test_wrap_codex.py --output-format concise
All checks passed!
uv run ruff format --check headroom/cli/recover.py headroom/providers/codex/recovery.py tests/test_cli/test_recover_codex.py tests/test_cli/test_wrap_codex.py
4 files already formatted
Description
Closes #2159.
Codex wrappers currently launch against a disposable
CODEX_HOME, so session state created during a wrapped run can disappear when that temporary directory is removed. This change launches Codex against its durable home, keeps proxy routing process-local, and adds recovery for retained temporary homes and pinned recovery sources.Type of Change
Changes Made
CODEX_HOMEand apply routing through process-local config overrides after the actual proxy port is resolved.headroom recover codexwith automatic discovery, repeatable--source, preview, confirmation, retained backups, and rollback on failure.$TMPDIR,/tmp,/private/tmp, and macOS/private/var/folders/*/*/Tfor retainedheadroom-codex-home-*directories.source-pinned/copies left by interrupted or failed recovery attempts after the original temporary home has disappeared.headroomproviders in both SQLite thread rows and rolloutsession_meta, including retries after an earlier broken recovery, while preserving user-defined remote providers namedheadroom.Testing
pytest)ruff check)mypy)Test Output
All validation ran in
ghcr.io/astral-sh/uv:python3.12-bookwormagainst a writable disposable copy of a read-only source mount. Codex was not installed or launched, and no real user Codex state was read or modified.The tests cover multi-root discovery, deleted-reference reporting, retained pinned-source recovery, durable SQLite path relocation, SQLite and rollout provider normalization, idempotent repair after an earlier broken recovery, remote provider preservation, unrelated dangling target rows, backup retention, atomic rollback, malformed-state quarantine, SQLite validation, and Windows-safe handle closure.
The repository shim E2E was not launched locally because this recovery work intentionally avoids launching Codex. Upstream CI exercises wrapper E2E in isolated environments.
Real Behavior Proof
ghcr.io/astral-sh/uv:python3.12-bookworm, Python 3.12, a writable disposable checkout copied from a read-only source mount, at head2d89ecec.pytest -q tests/test_cli/test_wrap_codex.py tests/test_cli/test_recover_codex.py, then runruff checkandruff format --checkagainstheadroom/cli/wrap.py,headroom/cli/recover.py,headroom/providers/codex/recovery.py,tests/test_cli/test_wrap_codex.py, andtests/test_cli/test_recover_codex.py.122 passed in 10.08s; Ruff reportedAll checks passed!and5 files already formatted.CODEX_HOME; these were intentionally excluded to protect live user state.Review Readiness
Checklist
CHANGELOG.mdif applicableAdditional Notes
The temporary-home behavior was introduced by #1507 in
ad9d086f43a664c4c2a19060b847f2e03ce4f6ad. Related context: #730, #731, #961, #1034, #1050, #1349, #1853, #1889, #2103, and #2104.A temporary home that macOS or
TemporaryDirectoryalready deleted cannot be reconstructed unless a retainedsource-pinned/copy exists. Recovery identifies genuine dangling SQLite paths, audits surviving durable history, and recovers any retained pinned source it can find. Prompt text without a rollout cannot reconstruct a full transcript.The unchecked changelog item is not applicable because this repository does not require a changelog entry for this fix.